Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(material/autocomplete): don't assign to model value while typing when requireSelection is enabled #27572

Merged

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Aug 3, 2023

Follow-up to #27423 based on the feedback. Usually mat-autocomplete assigns to the model as the user is typing which may not be desired when requireSelection is enabled, because at the end of the selection either an option value will set or it'll be reset.

These changes add a condition so that the value isn't assigned while typing and requireSelection is enabled.

@crisbeto crisbeto added P2 The issue is important to a large percentage of users, with a workaround target: rc This PR is targeted for the next release-candidate labels Aug 3, 2023
@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed action: merge The PR is ready for merge by the caretaker labels Aug 3, 2023
@crisbeto
Copy link
Member Author

crisbeto commented Aug 3, 2023

I had to add and remove the merge label to verify that the change will be merged in the right branch.

…when requireSelection is enabled

Follow-up to angular#27423 based on the feedback. Usually `mat-autocomplete` assigns to the model as the user is typing which may not be desired when `requireSelection` is enabled, because at the end of the selection either an option value will set or it'll be reset.

These changes add a condition so that the value isn't assigned while typing and `requireSelection` is enabled.
@crisbeto crisbeto force-pushed the 27423/require-selection-model-typing branch from 2a775d8 to e342192 Compare August 3, 2023 07:25
@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label Aug 3, 2023
@crisbeto crisbeto merged commit 77ffdf9 into angular:main Aug 3, 2023
13 checks passed
crisbeto added a commit that referenced this pull request Aug 3, 2023
…when requireSelection is enabled (#27572)

Follow-up to #27423 based on the feedback. Usually `mat-autocomplete` assigns to the model as the user is typing which may not be desired when `requireSelection` is enabled, because at the end of the selection either an option value will set or it'll be reset.

These changes add a condition so that the value isn't assigned while typing and `requireSelection` is enabled.

(cherry picked from commit 77ffdf9)
@mozgor
Copy link

mozgor commented Aug 10, 2023

Regarding your early comments in previous thread and especially this one, this fix means that despite being against the generic expectations of not interacting directly with the DOM while dealing with reactive forms (quoting you) this is the actual way of filtering autocomplete options ? I understand the reasoning behind this fix, but I'd double check before propagating updated guidelines inside our project.

@crisbeto
Copy link
Member Author

Yes, I had to make the tradeoff because it would've been a weird experience if the form control was emitting events with the string value while the user is typing and then a null event at the end if they don't select something since there's no way to distinguish where the value is coming from.

@mozgor
Copy link

mozgor commented Aug 10, 2023

Make sense, even if it would have been easier for my use case to filter out null value rather than switching source. Controls "valuesChanges" stream was very convenient especially to chain filter / debounce operators.

I'll figure that out, thanks for the confirmation.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker P2 The issue is important to a large percentage of users, with a workaround target: rc This PR is targeted for the next release-candidate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants